[analyzer] Fix crash on compound literals with bitfields in unions #146418
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Baranov Victor (vbvictor) ChangesFix a crash in Fixes #146050. Full diff: https://github.com/llvm/llvm-project/pull/146418.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index bee71cf296ac3..31cc160b8ae85 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1100,6 +1100,9 @@ Crash and bug fixes
- Fixed a crash in ``UnixAPIMisuseChecker`` and ``MallocChecker`` when analyzing
code with non-standard ``getline`` or ``getdelim`` function signatures. (#GH144884)
+- Fixed a crash when analyzing default bindings as compound literals in
+ designated initializers for bitfields in unions. (#GH146050)
+
Improvements
^^^^^^^^^^^^
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 388034b087789..288c53b375f28 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2187,7 +2187,10 @@ std::optional<SVal> RegionStoreManager::getBindingForDerivedDefaultValue(
// Lazy bindings are usually handled through getExistingLazyBinding().
// We should unify these two code paths at some point.
- if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal>(val))
+ // 'nonloc::ConcreteInt' values can arise from compound literals in
+ // designated initializers for bitfields in unions.
+ if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal, nonloc::ConcreteInt>(
+ val))
return val;
llvm_unreachable("Unknown default value");
diff --git a/clang/test/Analysis/fields.c b/clang/test/Analysis/fields.c
index 203c30c5960a1..66c882cdba1f3 100644
--- a/clang/test/Analysis/fields.c
+++ b/clang/test/Analysis/fields.c
@@ -113,6 +113,102 @@ void testBitfields(void) {
}
+struct BitfieldUnion {
+ union {
+ struct {
+ unsigned int addr : 22;
+ unsigned int vf : 1;
+ };
+ unsigned int raw;
+ };
+};
+
+struct BitfieldUnion processBitfieldUnion(struct BitfieldUnion r) {
+ struct BitfieldUnion result = r;
+ result.addr += 1;
+ return result;
+}
+
+void testBitfieldUnionCompoundLiteral(void) {
+ struct BitfieldUnion r1 = processBitfieldUnion((struct BitfieldUnion){.addr = 100, .vf = 1});
+ clang_analyzer_eval(r1.addr == 101); // expected-warning{{TRUE}}
+ clang_analyzer_eval(r1.vf == 1); // expected-warning{{UNKNOWN}}
+
+ struct BitfieldUnion r2 = processBitfieldUnion((struct BitfieldUnion){.addr = 1});
+ clang_analyzer_eval(r2.addr == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(r2.vf); // expected-warning{{UNKNOWN}}
+}
+
+struct NestedBitfields {
+ struct {
+ unsigned x : 16;
+ unsigned y : 16;
+ } inner;
+};
+
+struct NestedBitfields processNestedBitfields(struct NestedBitfields n) {
+ struct NestedBitfields tmp = n;
+ tmp.inner.x += 1;
+ return tmp;
+}
+
+void testNestedBitfields(void) {
+ struct NestedBitfields n1 = processNestedBitfields((struct NestedBitfields){.inner.x = 1});
+ clang_analyzer_eval(n1.inner.x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(n1.inner.y == 0); // expected-warning{{TRUE}}
+
+ struct NestedBitfields n2 = processNestedBitfields((struct NestedBitfields){{200, 300}});
+ clang_analyzer_eval(n2.inner.x == 201); // expected-warning{{TRUE}}
+ clang_analyzer_eval(n2.inner.y == 300); // expected-warning{{TRUE}}
+}
+
+struct UnionContainerBitfields {
+ union {
+ unsigned val;
+ struct {
+ unsigned x : 16;
+ unsigned y : 16;
+ };
+ } u;
+};
+
+struct UnionContainerBitfields processUnionContainer(struct UnionContainerBitfields c) {
+ struct UnionContainerBitfields tmp = c;
+ tmp.u.x += 1;
+ return tmp;
+}
+
+void testUnionContainerBitfields(void) {
+ struct UnionContainerBitfields c1 = processUnionContainer((struct UnionContainerBitfields){.u.val = 100});
+ clang_analyzer_eval(c1.u.x == 101); // expected-warning{{FALSE}} // expected-warning{{TRUE}}
+
+ struct UnionContainerBitfields c2 = processUnionContainer((struct UnionContainerBitfields){.u.x = 100});
+ clang_analyzer_eval(c2.u.x == 101); // expected-warning{{TRUE}}
+}
+
+struct MixedBitfields {
+ unsigned char x;
+ unsigned y : 12;
+ unsigned z : 20;
+};
+
+struct MixedBitfields processMixedBitfields(struct MixedBitfields m) {
+ struct MixedBitfields tmp = m;
+ tmp.y += 1;
+ return tmp;
+}
+
+void testMixedBitfields(void) {
+ struct MixedBitfields m1 = processMixedBitfields((struct MixedBitfields){.x = 100, .y = 100});
+ clang_analyzer_eval(m1.x == 100); // expected-warning{{TRUE}}
+ clang_analyzer_eval(m1.y == 101); // expected-warning{{TRUE}}
+
+ struct MixedBitfields m2 = processMixedBitfields((struct MixedBitfields){.z = 100});
+ clang_analyzer_eval(m2.y == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(m2.z == 100); // expected-warning{{TRUE}}
+}
+
+
//-----------------------------------------------------------------------------
// Incorrect behavior
//-----------------------------------------------------------------------------
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Baranov Victor (vbvictor) ChangesFix a crash in Fixes #146050. Full diff: https://github.com/llvm/llvm-project/pull/146418.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index bee71cf296ac3..31cc160b8ae85 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1100,6 +1100,9 @@ Crash and bug fixes
- Fixed a crash in ``UnixAPIMisuseChecker`` and ``MallocChecker`` when analyzing
code with non-standard ``getline`` or ``getdelim`` function signatures. (#GH144884)
+- Fixed a crash when analyzing default bindings as compound literals in
+ designated initializers for bitfields in unions. (#GH146050)
+
Improvements
^^^^^^^^^^^^
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 388034b087789..288c53b375f28 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2187,7 +2187,10 @@ std::optional<SVal> RegionStoreManager::getBindingForDerivedDefaultValue(
// Lazy bindings are usually handled through getExistingLazyBinding().
// We should unify these two code paths at some point.
- if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal>(val))
+ // 'nonloc::ConcreteInt' values can arise from compound literals in
+ // designated initializers for bitfields in unions.
+ if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal, nonloc::ConcreteInt>(
+ val))
return val;
llvm_unreachable("Unknown default value");
diff --git a/clang/test/Analysis/fields.c b/clang/test/Analysis/fields.c
index 203c30c5960a1..66c882cdba1f3 100644
--- a/clang/test/Analysis/fields.c
+++ b/clang/test/Analysis/fields.c
@@ -113,6 +113,102 @@ void testBitfields(void) {
}
+struct BitfieldUnion {
+ union {
+ struct {
+ unsigned int addr : 22;
+ unsigned int vf : 1;
+ };
+ unsigned int raw;
+ };
+};
+
+struct BitfieldUnion processBitfieldUnion(struct BitfieldUnion r) {
+ struct BitfieldUnion result = r;
+ result.addr += 1;
+ return result;
+}
+
+void testBitfieldUnionCompoundLiteral(void) {
+ struct BitfieldUnion r1 = processBitfieldUnion((struct BitfieldUnion){.addr = 100, .vf = 1});
+ clang_analyzer_eval(r1.addr == 101); // expected-warning{{TRUE}}
+ clang_analyzer_eval(r1.vf == 1); // expected-warning{{UNKNOWN}}
+
+ struct BitfieldUnion r2 = processBitfieldUnion((struct BitfieldUnion){.addr = 1});
+ clang_analyzer_eval(r2.addr == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(r2.vf); // expected-warning{{UNKNOWN}}
+}
+
+struct NestedBitfields {
+ struct {
+ unsigned x : 16;
+ unsigned y : 16;
+ } inner;
+};
+
+struct NestedBitfields processNestedBitfields(struct NestedBitfields n) {
+ struct NestedBitfields tmp = n;
+ tmp.inner.x += 1;
+ return tmp;
+}
+
+void testNestedBitfields(void) {
+ struct NestedBitfields n1 = processNestedBitfields((struct NestedBitfields){.inner.x = 1});
+ clang_analyzer_eval(n1.inner.x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(n1.inner.y == 0); // expected-warning{{TRUE}}
+
+ struct NestedBitfields n2 = processNestedBitfields((struct NestedBitfields){{200, 300}});
+ clang_analyzer_eval(n2.inner.x == 201); // expected-warning{{TRUE}}
+ clang_analyzer_eval(n2.inner.y == 300); // expected-warning{{TRUE}}
+}
+
+struct UnionContainerBitfields {
+ union {
+ unsigned val;
+ struct {
+ unsigned x : 16;
+ unsigned y : 16;
+ };
+ } u;
+};
+
+struct UnionContainerBitfields processUnionContainer(struct UnionContainerBitfields c) {
+ struct UnionContainerBitfields tmp = c;
+ tmp.u.x += 1;
+ return tmp;
+}
+
+void testUnionContainerBitfields(void) {
+ struct UnionContainerBitfields c1 = processUnionContainer((struct UnionContainerBitfields){.u.val = 100});
+ clang_analyzer_eval(c1.u.x == 101); // expected-warning{{FALSE}} // expected-warning{{TRUE}}
+
+ struct UnionContainerBitfields c2 = processUnionContainer((struct UnionContainerBitfields){.u.x = 100});
+ clang_analyzer_eval(c2.u.x == 101); // expected-warning{{TRUE}}
+}
+
+struct MixedBitfields {
+ unsigned char x;
+ unsigned y : 12;
+ unsigned z : 20;
+};
+
+struct MixedBitfields processMixedBitfields(struct MixedBitfields m) {
+ struct MixedBitfields tmp = m;
+ tmp.y += 1;
+ return tmp;
+}
+
+void testMixedBitfields(void) {
+ struct MixedBitfields m1 = processMixedBitfields((struct MixedBitfields){.x = 100, .y = 100});
+ clang_analyzer_eval(m1.x == 100); // expected-warning{{TRUE}}
+ clang_analyzer_eval(m1.y == 101); // expected-warning{{TRUE}}
+
+ struct MixedBitfields m2 = processMixedBitfields((struct MixedBitfields){.z = 100});
+ clang_analyzer_eval(m2.y == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(m2.z == 100); // expected-warning{{TRUE}}
+}
+
+
//-----------------------------------------------------------------------------
// Incorrect behavior
//-----------------------------------------------------------------------------
|
steakhal
reviewed
Jul 2, 2025
Comment on lines
2190
to
2193
| // 'nonloc::ConcreteInt' values can arise from compound literals in | ||
| // designated initializers for bitfields in unions. | ||
| if (isa<nonloc::LazyCompoundVal, nonloc::CompoundVal, nonloc::ConcreteInt>( | ||
| val)) |
There was a problem hiding this comment.
Could you help me with a Store for which the crash happened without the fix?
It would help me understand why do we have there a ConcreteInt, because I fear, if a ConcreteInt can get here, it's likely that other NonLocs can also reach this.
Anyways, LCVs and CompoundVals form one set of exception, and the ConcreteInt case is definitely something else thus shouldn't be handled by the same if but rather split into a separate if IMO.
There was a problem hiding this comment.
Could you help me with a Store for which the crash happened without the fix?
I'm not sure what do you mean by "a Store", minimal repro goes like this: https://godbolt.org/z/8z1vqf6Wv.
We get the crush as soon as try to get default binding when encountered a copied struct.
The fix may be a little "light-minded", I'll try to investigate more on the issue with other NonLocs that could get beside ConcreteInt. Also, can some deeper logic be broken, and we should never expect ConcreteInt right here?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix a crash in
RegionStore::getBindingForDerivedDefaultValuewhen analyzing compound literals with designated initializers for bitfields in unions. The crash occurred becausenonloc::ConcreteIntvalueswere not handled as possible default bindings.
Fixes #146050.